Skip to content

Conversation

@lenary
Copy link
Member

@lenary lenary commented Dec 20, 2024

This change aims to bring a lot of the immediate handling code, especially in TableGen, to become more uniform, both for different sizes, and also for unsigned vs signed immediates.

Some changes, none of which should affect current behaviour:

  • Both RISCVUImmOp and RISCVSImmOp now use getImmOpValue - which now has to return a 64-bit value because we have some 64-bit unsigned immediates for .insn
  • Both RISCVUImmOp and RISCVSImmOp now define a default MCOperandPredicate, based on their width, which just checks their value with isInt or isUInt. This is overridden for the immediates that accept a bare symbol ref in addition to an immediate. This allows some duplicate definitions to be removed.
  • I moved some OPERAND_ enum values around so that the unsigned immediates are beside each other, and so too for the signed immediates.

This change aims to bring a lot of the immediate handling code,
especially in TableGen, to become more uniform, both for different
sizes, and also for unsigned vs signed immediates.

Some changes, none of which should affect current behaviour:
- Both `RISCVUImmOp` and `RISCVSImmOp` now use getImmOpValue - which now
  has to return a 64-bit value because we have some 64-bit unsigned
  immediates for `.insn`
- Both `RISCVUImmOp` and `RISCVSImmOp` now define a default
  MCOperandPredicate, based on their width, which just checks their
  value with `isInt` or `isUInt`. This is overridden for the immediates
  that accept a bare symbol ref in addition to an immediate. This allows
  some duplicate definitions to be removed.
- I moved some OPERAND_ enum values around so that the unsigned
  immediates are beside each other, and so too for the signed
  immediates.
@lenary lenary added backend:RISC-V llvm:mc Machine (object) code labels Dec 20, 2024
@lenary lenary requested review from dtcxzyw and topperc December 20, 2024 11:47
@llvmbot
Copy link
Member

llvmbot commented Dec 20, 2024

@llvm/pr-subscribers-backend-risc-v

Author: Sam Elliott (lenary)

Changes

This change aims to bring a lot of the immediate handling code, especially in TableGen, to become more uniform, both for different sizes, and also for unsigned vs signed immediates.

Some changes, none of which should affect current behaviour:

  • Both RISCVUImmOp and RISCVSImmOp now use getImmOpValue - which now has to return a 64-bit value because we have some 64-bit unsigned immediates for .insn
  • Both RISCVUImmOp and RISCVSImmOp now define a default MCOperandPredicate, based on their width, which just checks their value with isInt or isUInt. This is overridden for the immediates that accept a bare symbol ref in addition to an immediate. This allows some duplicate definitions to be removed.
  • I moved some OPERAND_ enum values around so that the unsigned immediates are beside each other, and so too for the signed immediates.

Full diff: https://github.com/llvm/llvm-project/pull/120718.diff

7 Files Affected:

  • (modified) llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h (+3-3)
  • (modified) llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp (+5-5)
  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfo.cpp (+9-9)
  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfo.td (+17-19)
  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfoC.td (+1-4)
  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfoV.td (+1-7)
  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfoXwch.td (+12-30)
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h
index 056b6da34ac708..61dc9754afb512 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h
@@ -316,6 +316,9 @@ enum OperandType : unsigned {
   OPERAND_UIMM11,
   OPERAND_UIMM12,
   OPERAND_UIMM16,
+  OPERAND_UIMM20,
+  OPERAND_UIMMLOG2XLEN,
+  OPERAND_UIMMLOG2XLEN_NONZERO,
   OPERAND_UIMM32,
   OPERAND_UIMM48,
   OPERAND_UIMM64,
@@ -327,9 +330,6 @@ enum OperandType : unsigned {
   OPERAND_SIMM10_LSB0000_NONZERO,
   OPERAND_SIMM12,
   OPERAND_SIMM12_LSB00000,
-  OPERAND_UIMM20,
-  OPERAND_UIMMLOG2XLEN,
-  OPERAND_UIMMLOG2XLEN_NONZERO,
   OPERAND_CLUI_IMM,
   OPERAND_VTYPEI10,
   OPERAND_VTYPEI11,
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
index a28bf1186589d9..06d9cce48692a5 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
@@ -80,11 +80,11 @@ class RISCVMCCodeEmitter : public MCCodeEmitter {
                              SmallVectorImpl<MCFixup> &Fixups,
                              const MCSubtargetInfo &STI) const;
 
-  unsigned getImmOpValueAsr1(const MCInst &MI, unsigned OpNo,
+  uint64_t getImmOpValueAsr1(const MCInst &MI, unsigned OpNo,
                              SmallVectorImpl<MCFixup> &Fixups,
                              const MCSubtargetInfo &STI) const;
 
-  unsigned getImmOpValue(const MCInst &MI, unsigned OpNo,
+  uint64_t getImmOpValue(const MCInst &MI, unsigned OpNo,
                          SmallVectorImpl<MCFixup> &Fixups,
                          const MCSubtargetInfo &STI) const;
 
@@ -385,14 +385,14 @@ RISCVMCCodeEmitter::getMachineOpValue(const MCInst &MI, const MCOperand &MO,
   return 0;
 }
 
-unsigned
+uint64_t
 RISCVMCCodeEmitter::getImmOpValueAsr1(const MCInst &MI, unsigned OpNo,
                                       SmallVectorImpl<MCFixup> &Fixups,
                                       const MCSubtargetInfo &STI) const {
   const MCOperand &MO = MI.getOperand(OpNo);
 
   if (MO.isImm()) {
-    unsigned Res = MO.getImm();
+    uint64_t Res = MO.getImm();
     assert((Res & 1) == 0 && "LSB is non-zero");
     return Res >> 1;
   }
@@ -400,7 +400,7 @@ RISCVMCCodeEmitter::getImmOpValueAsr1(const MCInst &MI, unsigned OpNo,
   return getImmOpValue(MI, OpNo, Fixups, STI);
 }
 
-unsigned RISCVMCCodeEmitter::getImmOpValue(const MCInst &MI, unsigned OpNo,
+uint64_t RISCVMCCodeEmitter::getImmOpValue(const MCInst &MI, unsigned OpNo,
                                            SmallVectorImpl<MCFixup> &Fixups,
                                            const MCSubtargetInfo &STI) const {
   bool EnableRelax = STI.hasFeature(RISCV::FeatureRelax);
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
index 0af8161a307abd..ee720ceb22b00f 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
@@ -2466,6 +2466,10 @@ bool RISCVInstrInfo::verifyInstruction(const MachineInstr &MI,
 #define CASE_OPERAND_UIMM(NUM)                                                 \
   case RISCVOp::OPERAND_UIMM##NUM:                                             \
     Ok = isUInt<NUM>(Imm);                                                     \
+    break;
+#define CASE_OPERAND_SIMM(NUM)                                                 \
+  case RISCVOp::OPERAND_SIMM##NUM:                                             \
+    Ok = isInt<NUM>(Imm);                                                      \
     break;
         CASE_OPERAND_UIMM(1)
         CASE_OPERAND_UIMM(2)
@@ -2511,15 +2515,14 @@ bool RISCVInstrInfo::verifyInstruction(const MachineInstr &MI,
         case RISCVOp::OPERAND_ZERO:
           Ok = Imm == 0;
           break;
-        case RISCVOp::OPERAND_SIMM5:
-          Ok = isInt<5>(Imm);
-          break;
+          // clang-format off
+        CASE_OPERAND_SIMM(5)
+        CASE_OPERAND_SIMM(6)
+        CASE_OPERAND_SIMM(12)
+        // clang-format on
         case RISCVOp::OPERAND_SIMM5_PLUS1:
           Ok = (isInt<5>(Imm) && Imm != -16) || Imm == 16;
           break;
-        case RISCVOp::OPERAND_SIMM6:
-          Ok = isInt<6>(Imm);
-          break;
         case RISCVOp::OPERAND_SIMM6_NONZERO:
           Ok = Imm != 0 && isInt<6>(Imm);
           break;
@@ -2529,9 +2532,6 @@ bool RISCVInstrInfo::verifyInstruction(const MachineInstr &MI,
         case RISCVOp::OPERAND_VTYPEI11:
           Ok = isUInt<11>(Imm);
           break;
-        case RISCVOp::OPERAND_SIMM12:
-          Ok = isInt<12>(Imm);
-          break;
         case RISCVOp::OPERAND_SIMM12_LSB00000:
           Ok = isShiftedInt<7, 5>(Imm);
           break;
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.td b/llvm/lib/Target/RISCV/RISCVInstrInfo.td
index 24bf7da4dadc07..13f536affb03b7 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.td
@@ -159,8 +159,15 @@ class RISCVOp<ValueType vt = XLenVT> : Operand<vt> {
 
 class RISCVUImmOp<int bitsNum> : RISCVOp {
   let ParserMatchClass = UImmAsmOperand<bitsNum>;
+  let EncoderMethod = "getImmOpValue";
   let DecoderMethod = "decodeUImmOperand<" # bitsNum # ">";
   let OperandType = "OPERAND_UIMM" # bitsNum;
+  let MCOperandPredicate = [{
+    int64_t Imm;
+    if (!MCOp.evaluateAsConstantImm(Imm))
+      return false;
+    return isUInt<}]# bitsNum #[{>(Imm);
+  }];
 }
 
 class RISCVUImmLeafOp<int bitsNum> :
@@ -171,6 +178,12 @@ class RISCVSImmOp<int bitsNum> : RISCVOp {
   let EncoderMethod = "getImmOpValue";
   let DecoderMethod = "decodeSImmOperand<" # bitsNum # ">";
   let OperandType = "OPERAND_SIMM" # bitsNum;
+  let MCOperandPredicate = [{
+    int64_t Imm;
+    if (!MCOp.evaluateAsConstantImm(Imm))
+      return false;
+    return isInt<}] # bitsNum # [{>(Imm);
+  }];
 }
 
 class RISCVSImmLeafOp<int bitsNum> :
@@ -221,16 +234,9 @@ def InsnDirectiveOpcode : AsmOperandClass {
 }
 
 def uimm1 : RISCVUImmLeafOp<1>;
-def uimm2 : RISCVUImmLeafOp<2> {
-  let MCOperandPredicate = [{
-    int64_t Imm;
-    if (!MCOp.evaluateAsConstantImm(Imm))
-      return false;
-    return isUInt<2>(Imm);
-  }];
-}
+def uimm2 : RISCVUImmLeafOp<2>;
 def uimm3 : RISCVUImmOp<3>;
-def uimm4 : RISCVUImmOp<4>;
+def uimm4 : RISCVUImmLeafOp<4>;
 def uimm5 : RISCVUImmLeafOp<5>;
 def uimm6 : RISCVUImmLeafOp<6>;
 def uimm7_opcode : RISCVUImmOp<7> {
@@ -277,7 +283,7 @@ class UImm20Operand : RISCVOp {
   let OperandType = "OPERAND_UIMM20";
 }
 
-class UImm20OperandMaybeSym : UImm20Operand {
+class UImm20OperandMaybeSym : RISCVUImmOp<20> {
   let MCOperandPredicate = [{
     int64_t Imm;
     if (MCOp.evaluateAsConstantImm(Imm))
@@ -293,15 +299,7 @@ def uimm20_auipc : UImm20OperandMaybeSym {
   let ParserMatchClass = UImmAsmOperand<20, "AUIPC">;
 }
 
-def uimm20 : UImm20Operand {
-  let ParserMatchClass = UImmAsmOperand<20>;
-  let MCOperandPredicate = [{
-    int64_t Imm;
-    if (!MCOp.evaluateAsConstantImm(Imm))
-      return false;
-    return isUInt<20>(Imm);
-  }];
-}
+def uimm20 : RISCVUImmOp<20>;
 
 def Simm21Lsb0JALAsmOperand : SImmAsmOperand<21, "Lsb0JAL"> {
   let ParserMethod = "parseJALOffset";
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoC.td b/llvm/lib/Target/RISCV/RISCVInstrInfoC.td
index e5a5f60f9fec10..ce994206cd785b 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoC.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoC.td
@@ -151,7 +151,6 @@ def simm9_lsb0 : Operand<OtherVT>,
     if (MCOp.evaluateAsConstantImm(Imm))
       return isShiftedInt<8, 1>(Imm);
     return MCOp.isBareSymbolRef();
-
   }];
   let OperandType = "OPERAND_PCREL";
 }
@@ -227,10 +226,8 @@ def InsnCDirectiveOpcode : AsmOperandClass {
   let PredicateMethod = "isImm";
 }
 
-def uimm2_opcode : RISCVOp {
+def uimm2_opcode : RISCVUImmOp<2> {
   let ParserMatchClass = InsnCDirectiveOpcode;
-  let DecoderMethod = "decodeUImmOperand<2>";
-  let OperandType = "OPERAND_UIMM2";
 }
 
 //===----------------------------------------------------------------------===//
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoV.td b/llvm/lib/Target/RISCV/RISCVInstrInfoV.td
index 6506b6746b1517..24a881dc6810f8 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoV.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoV.td
@@ -66,15 +66,9 @@ def simm5 : RISCVSImmLeafOp<5> {
   }];
 }
 
-def SImm5Plus1AsmOperand : AsmOperandClass {
-  let Name = "SImm5Plus1";
-  let RenderMethod = "addImmOperands";
-  let DiagnosticType = "InvalidSImm5Plus1";
-}
-
 def simm5_plus1 : RISCVOp, ImmLeaf<XLenVT,
   [{return (isInt<5>(Imm) && Imm != -16) || Imm == 16;}]> {
-  let ParserMatchClass = SImm5Plus1AsmOperand;
+  let ParserMatchClass = SImmAsmOperand<5, "Plus1">;
   let OperandType = "OPERAND_SIMM5_PLUS1";
   let MCOperandPredicate = [{
     int64_t Imm;
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoXwch.td b/llvm/lib/Target/RISCV/RISCVInstrInfoXwch.td
index 91ff804ba105ac..a43cbadf6f3080 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoXwch.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoXwch.td
@@ -25,24 +25,6 @@ class QKStackInst<bits<2> funct2, dag outs, dag ins,
 // Operand definitions.
 //===----------------------------------------------------------------------===//
 
-def uimm4_with_predicate : RISCVUImmLeafOp<4> {
-  let MCOperandPredicate = [{
-    int64_t Imm;
-    if (!MCOp.evaluateAsConstantImm(Imm))
-      return false;
-    return isUInt<4>(Imm);
-  }];
-}
-
-def uimm5_with_predicate : RISCVUImmLeafOp<5> {
-  let MCOperandPredicate = [{
-    int64_t Imm;
-    if (!MCOp.evaluateAsConstantImm(Imm))
-      return false;
-    return isUInt<5>(Imm);
-  }];
-}
-
 // A 5-bit unsigned immediate where the least significant bit is zero.
 def uimm5_lsb0 : RISCVOp,
                  ImmLeaf<XLenVT, [{return isShiftedUInt<4, 1>(Imm);}]> {
@@ -80,7 +62,7 @@ let Predicates = [HasVendorXwchc], DecoderNamespace = "Xwchc" in {
 
 let hasSideEffects = 0, mayLoad = 1, mayStore = 0 in
 def QK_C_LBU : RVInst16CL<0b001, 0b00, (outs GPRC:$rd),
-                          (ins GPRCMem:$rs1, uimm5_with_predicate:$imm),
+                          (ins GPRCMem:$rs1, uimm5:$imm),
                           "qk.c.lbu", "$rd, ${imm}(${rs1})">,
                Sched<[WriteLDB, ReadMemBase]> {
   bits<5> imm;
@@ -91,7 +73,7 @@ def QK_C_LBU : RVInst16CL<0b001, 0b00, (outs GPRC:$rd),
 let hasSideEffects = 0, mayLoad = 0, mayStore = 1 in
 def QK_C_SB : RVInst16CS<0b101, 0b00, (outs),
                          (ins GPRC:$rs2, GPRCMem:$rs1,
-                              uimm5_with_predicate:$imm),
+                              uimm5:$imm),
                          "qk.c.sb", "$rs2, ${imm}(${rs1})">,
               Sched<[WriteSTB, ReadStoreData, ReadMemBase]> {
   bits<5> imm;
@@ -121,7 +103,7 @@ def QK_C_SH : RVInst16CS<0b101, 0b10, (outs),
 
 let hasSideEffects = 0, mayLoad = 1, mayStore = 0 in
 def QK_C_LBUSP : QKStackInst<0b00, (outs GPRC:$rd_rs2),
-                             (ins SPMem:$rs1, uimm4_with_predicate:$imm),
+                             (ins SPMem:$rs1, uimm4:$imm),
                              "qk.c.lbusp", "$rd_rs2, ${imm}(${rs1})">,
                  Sched<[WriteLDB, ReadMemBase]> {
   bits<4> imm;
@@ -130,7 +112,7 @@ def QK_C_LBUSP : QKStackInst<0b00, (outs GPRC:$rd_rs2),
 let hasSideEffects = 0, mayLoad = 0, mayStore = 1 in
 def QK_C_SBSP : QKStackInst<0b10, (outs),
                             (ins GPRC:$rd_rs2, SPMem:$rs1,
-                                 uimm4_with_predicate:$imm),
+                                 uimm4:$imm),
                             "qk.c.sbsp", "$rd_rs2, ${imm}(${rs1})">,
                 Sched<[WriteSTB, ReadStoreData, ReadMemBase]> {
   bits<4> imm;
@@ -180,18 +162,18 @@ def : InstAlias<"qk.c.shsp $rs2, (${rs1})", (QK_C_SHSP GPRC:$rs2, SPMem:$rs1, 0)
 //===----------------------------------------------------------------------===//
 
 let Predicates = [HasVendorXwchc] in {
-def : CompressPat<(LBU GPRC:$rd, GPRCMem:$rs1, uimm5_with_predicate:$imm),
-                  (QK_C_LBU GPRC:$rd, GPRCMem:$rs1, uimm5_with_predicate:$imm)>;
-def : CompressPat<(SB GPRC:$rs2, GPRCMem:$rs1, uimm5_with_predicate:$imm),
-                  (QK_C_SB GPRC:$rs2, GPRCMem:$rs1, uimm5_with_predicate:$imm)>;
+def : CompressPat<(LBU GPRC:$rd, GPRCMem:$rs1, uimm5:$imm),
+                  (QK_C_LBU GPRC:$rd, GPRCMem:$rs1, uimm5:$imm)>;
+def : CompressPat<(SB GPRC:$rs2, GPRCMem:$rs1, uimm5:$imm),
+                  (QK_C_SB GPRC:$rs2, GPRCMem:$rs1, uimm5:$imm)>;
 def : CompressPat<(LHU GPRC:$rd, GPRCMem:$rs1, uimm6_lsb0:$imm),
                   (QK_C_LHU GPRC:$rd, GPRCMem:$rs1, uimm6_lsb0:$imm)>;
 def : CompressPat<(SH GPRC:$rs2, GPRCMem:$rs1, uimm6_lsb0:$imm),
                   (QK_C_SH GPRC:$rs2, GPRCMem:$rs1, uimm6_lsb0:$imm)>;
-def : CompressPat<(LBU GPRC:$rd, SPMem:$rs1,   uimm4_with_predicate:$imm),
-                  (QK_C_LBUSP GPRC:$rd, SPMem:$rs1, uimm4_with_predicate:$imm)>;
-def : CompressPat<(SB GPRC:$rs2, SPMem:$rs1,   uimm4_with_predicate:$imm),
-                  (QK_C_SBSP GPRC:$rs2, SPMem:$rs1, uimm4_with_predicate:$imm)>;
+def : CompressPat<(LBU GPRC:$rd, SPMem:$rs1,   uimm4:$imm),
+                  (QK_C_LBUSP GPRC:$rd, SPMem:$rs1, uimm4:$imm)>;
+def : CompressPat<(SB GPRC:$rs2, SPMem:$rs1,   uimm4:$imm),
+                  (QK_C_SBSP GPRC:$rs2, SPMem:$rs1, uimm4:$imm)>;
 def : CompressPat<(LHU GPRC:$rd, SPMem:$rs1,   uimm5_lsb0:$imm),
                   (QK_C_LHUSP GPRC:$rd, SPMem:$rs1, uimm5_lsb0:$imm)>;
 def : CompressPat<(SH GPRC:$rs2, SPMem:$rs1,   uimm5_lsb0:$imm),

@llvmbot
Copy link
Member

llvmbot commented Dec 20, 2024

@llvm/pr-subscribers-mc

Author: Sam Elliott (lenary)

Changes

This change aims to bring a lot of the immediate handling code, especially in TableGen, to become more uniform, both for different sizes, and also for unsigned vs signed immediates.

Some changes, none of which should affect current behaviour:

  • Both RISCVUImmOp and RISCVSImmOp now use getImmOpValue - which now has to return a 64-bit value because we have some 64-bit unsigned immediates for .insn
  • Both RISCVUImmOp and RISCVSImmOp now define a default MCOperandPredicate, based on their width, which just checks their value with isInt or isUInt. This is overridden for the immediates that accept a bare symbol ref in addition to an immediate. This allows some duplicate definitions to be removed.
  • I moved some OPERAND_ enum values around so that the unsigned immediates are beside each other, and so too for the signed immediates.

Full diff: https://github.com/llvm/llvm-project/pull/120718.diff

7 Files Affected:

  • (modified) llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h (+3-3)
  • (modified) llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp (+5-5)
  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfo.cpp (+9-9)
  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfo.td (+17-19)
  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfoC.td (+1-4)
  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfoV.td (+1-7)
  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfoXwch.td (+12-30)
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h
index 056b6da34ac708..61dc9754afb512 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h
@@ -316,6 +316,9 @@ enum OperandType : unsigned {
   OPERAND_UIMM11,
   OPERAND_UIMM12,
   OPERAND_UIMM16,
+  OPERAND_UIMM20,
+  OPERAND_UIMMLOG2XLEN,
+  OPERAND_UIMMLOG2XLEN_NONZERO,
   OPERAND_UIMM32,
   OPERAND_UIMM48,
   OPERAND_UIMM64,
@@ -327,9 +330,6 @@ enum OperandType : unsigned {
   OPERAND_SIMM10_LSB0000_NONZERO,
   OPERAND_SIMM12,
   OPERAND_SIMM12_LSB00000,
-  OPERAND_UIMM20,
-  OPERAND_UIMMLOG2XLEN,
-  OPERAND_UIMMLOG2XLEN_NONZERO,
   OPERAND_CLUI_IMM,
   OPERAND_VTYPEI10,
   OPERAND_VTYPEI11,
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
index a28bf1186589d9..06d9cce48692a5 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
@@ -80,11 +80,11 @@ class RISCVMCCodeEmitter : public MCCodeEmitter {
                              SmallVectorImpl<MCFixup> &Fixups,
                              const MCSubtargetInfo &STI) const;
 
-  unsigned getImmOpValueAsr1(const MCInst &MI, unsigned OpNo,
+  uint64_t getImmOpValueAsr1(const MCInst &MI, unsigned OpNo,
                              SmallVectorImpl<MCFixup> &Fixups,
                              const MCSubtargetInfo &STI) const;
 
-  unsigned getImmOpValue(const MCInst &MI, unsigned OpNo,
+  uint64_t getImmOpValue(const MCInst &MI, unsigned OpNo,
                          SmallVectorImpl<MCFixup> &Fixups,
                          const MCSubtargetInfo &STI) const;
 
@@ -385,14 +385,14 @@ RISCVMCCodeEmitter::getMachineOpValue(const MCInst &MI, const MCOperand &MO,
   return 0;
 }
 
-unsigned
+uint64_t
 RISCVMCCodeEmitter::getImmOpValueAsr1(const MCInst &MI, unsigned OpNo,
                                       SmallVectorImpl<MCFixup> &Fixups,
                                       const MCSubtargetInfo &STI) const {
   const MCOperand &MO = MI.getOperand(OpNo);
 
   if (MO.isImm()) {
-    unsigned Res = MO.getImm();
+    uint64_t Res = MO.getImm();
     assert((Res & 1) == 0 && "LSB is non-zero");
     return Res >> 1;
   }
@@ -400,7 +400,7 @@ RISCVMCCodeEmitter::getImmOpValueAsr1(const MCInst &MI, unsigned OpNo,
   return getImmOpValue(MI, OpNo, Fixups, STI);
 }
 
-unsigned RISCVMCCodeEmitter::getImmOpValue(const MCInst &MI, unsigned OpNo,
+uint64_t RISCVMCCodeEmitter::getImmOpValue(const MCInst &MI, unsigned OpNo,
                                            SmallVectorImpl<MCFixup> &Fixups,
                                            const MCSubtargetInfo &STI) const {
   bool EnableRelax = STI.hasFeature(RISCV::FeatureRelax);
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
index 0af8161a307abd..ee720ceb22b00f 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
@@ -2466,6 +2466,10 @@ bool RISCVInstrInfo::verifyInstruction(const MachineInstr &MI,
 #define CASE_OPERAND_UIMM(NUM)                                                 \
   case RISCVOp::OPERAND_UIMM##NUM:                                             \
     Ok = isUInt<NUM>(Imm);                                                     \
+    break;
+#define CASE_OPERAND_SIMM(NUM)                                                 \
+  case RISCVOp::OPERAND_SIMM##NUM:                                             \
+    Ok = isInt<NUM>(Imm);                                                      \
     break;
         CASE_OPERAND_UIMM(1)
         CASE_OPERAND_UIMM(2)
@@ -2511,15 +2515,14 @@ bool RISCVInstrInfo::verifyInstruction(const MachineInstr &MI,
         case RISCVOp::OPERAND_ZERO:
           Ok = Imm == 0;
           break;
-        case RISCVOp::OPERAND_SIMM5:
-          Ok = isInt<5>(Imm);
-          break;
+          // clang-format off
+        CASE_OPERAND_SIMM(5)
+        CASE_OPERAND_SIMM(6)
+        CASE_OPERAND_SIMM(12)
+        // clang-format on
         case RISCVOp::OPERAND_SIMM5_PLUS1:
           Ok = (isInt<5>(Imm) && Imm != -16) || Imm == 16;
           break;
-        case RISCVOp::OPERAND_SIMM6:
-          Ok = isInt<6>(Imm);
-          break;
         case RISCVOp::OPERAND_SIMM6_NONZERO:
           Ok = Imm != 0 && isInt<6>(Imm);
           break;
@@ -2529,9 +2532,6 @@ bool RISCVInstrInfo::verifyInstruction(const MachineInstr &MI,
         case RISCVOp::OPERAND_VTYPEI11:
           Ok = isUInt<11>(Imm);
           break;
-        case RISCVOp::OPERAND_SIMM12:
-          Ok = isInt<12>(Imm);
-          break;
         case RISCVOp::OPERAND_SIMM12_LSB00000:
           Ok = isShiftedInt<7, 5>(Imm);
           break;
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.td b/llvm/lib/Target/RISCV/RISCVInstrInfo.td
index 24bf7da4dadc07..13f536affb03b7 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.td
@@ -159,8 +159,15 @@ class RISCVOp<ValueType vt = XLenVT> : Operand<vt> {
 
 class RISCVUImmOp<int bitsNum> : RISCVOp {
   let ParserMatchClass = UImmAsmOperand<bitsNum>;
+  let EncoderMethod = "getImmOpValue";
   let DecoderMethod = "decodeUImmOperand<" # bitsNum # ">";
   let OperandType = "OPERAND_UIMM" # bitsNum;
+  let MCOperandPredicate = [{
+    int64_t Imm;
+    if (!MCOp.evaluateAsConstantImm(Imm))
+      return false;
+    return isUInt<}]# bitsNum #[{>(Imm);
+  }];
 }
 
 class RISCVUImmLeafOp<int bitsNum> :
@@ -171,6 +178,12 @@ class RISCVSImmOp<int bitsNum> : RISCVOp {
   let EncoderMethod = "getImmOpValue";
   let DecoderMethod = "decodeSImmOperand<" # bitsNum # ">";
   let OperandType = "OPERAND_SIMM" # bitsNum;
+  let MCOperandPredicate = [{
+    int64_t Imm;
+    if (!MCOp.evaluateAsConstantImm(Imm))
+      return false;
+    return isInt<}] # bitsNum # [{>(Imm);
+  }];
 }
 
 class RISCVSImmLeafOp<int bitsNum> :
@@ -221,16 +234,9 @@ def InsnDirectiveOpcode : AsmOperandClass {
 }
 
 def uimm1 : RISCVUImmLeafOp<1>;
-def uimm2 : RISCVUImmLeafOp<2> {
-  let MCOperandPredicate = [{
-    int64_t Imm;
-    if (!MCOp.evaluateAsConstantImm(Imm))
-      return false;
-    return isUInt<2>(Imm);
-  }];
-}
+def uimm2 : RISCVUImmLeafOp<2>;
 def uimm3 : RISCVUImmOp<3>;
-def uimm4 : RISCVUImmOp<4>;
+def uimm4 : RISCVUImmLeafOp<4>;
 def uimm5 : RISCVUImmLeafOp<5>;
 def uimm6 : RISCVUImmLeafOp<6>;
 def uimm7_opcode : RISCVUImmOp<7> {
@@ -277,7 +283,7 @@ class UImm20Operand : RISCVOp {
   let OperandType = "OPERAND_UIMM20";
 }
 
-class UImm20OperandMaybeSym : UImm20Operand {
+class UImm20OperandMaybeSym : RISCVUImmOp<20> {
   let MCOperandPredicate = [{
     int64_t Imm;
     if (MCOp.evaluateAsConstantImm(Imm))
@@ -293,15 +299,7 @@ def uimm20_auipc : UImm20OperandMaybeSym {
   let ParserMatchClass = UImmAsmOperand<20, "AUIPC">;
 }
 
-def uimm20 : UImm20Operand {
-  let ParserMatchClass = UImmAsmOperand<20>;
-  let MCOperandPredicate = [{
-    int64_t Imm;
-    if (!MCOp.evaluateAsConstantImm(Imm))
-      return false;
-    return isUInt<20>(Imm);
-  }];
-}
+def uimm20 : RISCVUImmOp<20>;
 
 def Simm21Lsb0JALAsmOperand : SImmAsmOperand<21, "Lsb0JAL"> {
   let ParserMethod = "parseJALOffset";
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoC.td b/llvm/lib/Target/RISCV/RISCVInstrInfoC.td
index e5a5f60f9fec10..ce994206cd785b 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoC.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoC.td
@@ -151,7 +151,6 @@ def simm9_lsb0 : Operand<OtherVT>,
     if (MCOp.evaluateAsConstantImm(Imm))
       return isShiftedInt<8, 1>(Imm);
     return MCOp.isBareSymbolRef();
-
   }];
   let OperandType = "OPERAND_PCREL";
 }
@@ -227,10 +226,8 @@ def InsnCDirectiveOpcode : AsmOperandClass {
   let PredicateMethod = "isImm";
 }
 
-def uimm2_opcode : RISCVOp {
+def uimm2_opcode : RISCVUImmOp<2> {
   let ParserMatchClass = InsnCDirectiveOpcode;
-  let DecoderMethod = "decodeUImmOperand<2>";
-  let OperandType = "OPERAND_UIMM2";
 }
 
 //===----------------------------------------------------------------------===//
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoV.td b/llvm/lib/Target/RISCV/RISCVInstrInfoV.td
index 6506b6746b1517..24a881dc6810f8 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoV.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoV.td
@@ -66,15 +66,9 @@ def simm5 : RISCVSImmLeafOp<5> {
   }];
 }
 
-def SImm5Plus1AsmOperand : AsmOperandClass {
-  let Name = "SImm5Plus1";
-  let RenderMethod = "addImmOperands";
-  let DiagnosticType = "InvalidSImm5Plus1";
-}
-
 def simm5_plus1 : RISCVOp, ImmLeaf<XLenVT,
   [{return (isInt<5>(Imm) && Imm != -16) || Imm == 16;}]> {
-  let ParserMatchClass = SImm5Plus1AsmOperand;
+  let ParserMatchClass = SImmAsmOperand<5, "Plus1">;
   let OperandType = "OPERAND_SIMM5_PLUS1";
   let MCOperandPredicate = [{
     int64_t Imm;
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoXwch.td b/llvm/lib/Target/RISCV/RISCVInstrInfoXwch.td
index 91ff804ba105ac..a43cbadf6f3080 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoXwch.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoXwch.td
@@ -25,24 +25,6 @@ class QKStackInst<bits<2> funct2, dag outs, dag ins,
 // Operand definitions.
 //===----------------------------------------------------------------------===//
 
-def uimm4_with_predicate : RISCVUImmLeafOp<4> {
-  let MCOperandPredicate = [{
-    int64_t Imm;
-    if (!MCOp.evaluateAsConstantImm(Imm))
-      return false;
-    return isUInt<4>(Imm);
-  }];
-}
-
-def uimm5_with_predicate : RISCVUImmLeafOp<5> {
-  let MCOperandPredicate = [{
-    int64_t Imm;
-    if (!MCOp.evaluateAsConstantImm(Imm))
-      return false;
-    return isUInt<5>(Imm);
-  }];
-}
-
 // A 5-bit unsigned immediate where the least significant bit is zero.
 def uimm5_lsb0 : RISCVOp,
                  ImmLeaf<XLenVT, [{return isShiftedUInt<4, 1>(Imm);}]> {
@@ -80,7 +62,7 @@ let Predicates = [HasVendorXwchc], DecoderNamespace = "Xwchc" in {
 
 let hasSideEffects = 0, mayLoad = 1, mayStore = 0 in
 def QK_C_LBU : RVInst16CL<0b001, 0b00, (outs GPRC:$rd),
-                          (ins GPRCMem:$rs1, uimm5_with_predicate:$imm),
+                          (ins GPRCMem:$rs1, uimm5:$imm),
                           "qk.c.lbu", "$rd, ${imm}(${rs1})">,
                Sched<[WriteLDB, ReadMemBase]> {
   bits<5> imm;
@@ -91,7 +73,7 @@ def QK_C_LBU : RVInst16CL<0b001, 0b00, (outs GPRC:$rd),
 let hasSideEffects = 0, mayLoad = 0, mayStore = 1 in
 def QK_C_SB : RVInst16CS<0b101, 0b00, (outs),
                          (ins GPRC:$rs2, GPRCMem:$rs1,
-                              uimm5_with_predicate:$imm),
+                              uimm5:$imm),
                          "qk.c.sb", "$rs2, ${imm}(${rs1})">,
               Sched<[WriteSTB, ReadStoreData, ReadMemBase]> {
   bits<5> imm;
@@ -121,7 +103,7 @@ def QK_C_SH : RVInst16CS<0b101, 0b10, (outs),
 
 let hasSideEffects = 0, mayLoad = 1, mayStore = 0 in
 def QK_C_LBUSP : QKStackInst<0b00, (outs GPRC:$rd_rs2),
-                             (ins SPMem:$rs1, uimm4_with_predicate:$imm),
+                             (ins SPMem:$rs1, uimm4:$imm),
                              "qk.c.lbusp", "$rd_rs2, ${imm}(${rs1})">,
                  Sched<[WriteLDB, ReadMemBase]> {
   bits<4> imm;
@@ -130,7 +112,7 @@ def QK_C_LBUSP : QKStackInst<0b00, (outs GPRC:$rd_rs2),
 let hasSideEffects = 0, mayLoad = 0, mayStore = 1 in
 def QK_C_SBSP : QKStackInst<0b10, (outs),
                             (ins GPRC:$rd_rs2, SPMem:$rs1,
-                                 uimm4_with_predicate:$imm),
+                                 uimm4:$imm),
                             "qk.c.sbsp", "$rd_rs2, ${imm}(${rs1})">,
                 Sched<[WriteSTB, ReadStoreData, ReadMemBase]> {
   bits<4> imm;
@@ -180,18 +162,18 @@ def : InstAlias<"qk.c.shsp $rs2, (${rs1})", (QK_C_SHSP GPRC:$rs2, SPMem:$rs1, 0)
 //===----------------------------------------------------------------------===//
 
 let Predicates = [HasVendorXwchc] in {
-def : CompressPat<(LBU GPRC:$rd, GPRCMem:$rs1, uimm5_with_predicate:$imm),
-                  (QK_C_LBU GPRC:$rd, GPRCMem:$rs1, uimm5_with_predicate:$imm)>;
-def : CompressPat<(SB GPRC:$rs2, GPRCMem:$rs1, uimm5_with_predicate:$imm),
-                  (QK_C_SB GPRC:$rs2, GPRCMem:$rs1, uimm5_with_predicate:$imm)>;
+def : CompressPat<(LBU GPRC:$rd, GPRCMem:$rs1, uimm5:$imm),
+                  (QK_C_LBU GPRC:$rd, GPRCMem:$rs1, uimm5:$imm)>;
+def : CompressPat<(SB GPRC:$rs2, GPRCMem:$rs1, uimm5:$imm),
+                  (QK_C_SB GPRC:$rs2, GPRCMem:$rs1, uimm5:$imm)>;
 def : CompressPat<(LHU GPRC:$rd, GPRCMem:$rs1, uimm6_lsb0:$imm),
                   (QK_C_LHU GPRC:$rd, GPRCMem:$rs1, uimm6_lsb0:$imm)>;
 def : CompressPat<(SH GPRC:$rs2, GPRCMem:$rs1, uimm6_lsb0:$imm),
                   (QK_C_SH GPRC:$rs2, GPRCMem:$rs1, uimm6_lsb0:$imm)>;
-def : CompressPat<(LBU GPRC:$rd, SPMem:$rs1,   uimm4_with_predicate:$imm),
-                  (QK_C_LBUSP GPRC:$rd, SPMem:$rs1, uimm4_with_predicate:$imm)>;
-def : CompressPat<(SB GPRC:$rs2, SPMem:$rs1,   uimm4_with_predicate:$imm),
-                  (QK_C_SBSP GPRC:$rs2, SPMem:$rs1, uimm4_with_predicate:$imm)>;
+def : CompressPat<(LBU GPRC:$rd, SPMem:$rs1,   uimm4:$imm),
+                  (QK_C_LBUSP GPRC:$rd, SPMem:$rs1, uimm4:$imm)>;
+def : CompressPat<(SB GPRC:$rs2, SPMem:$rs1,   uimm4:$imm),
+                  (QK_C_SBSP GPRC:$rs2, SPMem:$rs1, uimm4:$imm)>;
 def : CompressPat<(LHU GPRC:$rd, SPMem:$rs1,   uimm5_lsb0:$imm),
                   (QK_C_LHUSP GPRC:$rd, SPMem:$rs1, uimm5_lsb0:$imm)>;
 def : CompressPat<(SH GPRC:$rs2, SPMem:$rs1,   uimm5_lsb0:$imm),

class UImm20Operand : RISCVOp {
let EncoderMethod = "getImmOpValue";
let DecoderMethod = "decodeUImmOperand<20>";
let OperandType = "OPERAND_UIMM20";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UImm20Operand is unused now.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed, I knew I'd missed something :)

Copy link
Collaborator

@topperc topperc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lenary lenary merged commit 95c5386 into llvm:main Dec 21, 2024
8 checks passed
@lenary lenary deleted the pr/rationalize-imms branch December 21, 2024 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend:RISC-V llvm:mc Machine (object) code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants